refactor: add missing explicit (non-dynamic) imports silence linter errors#719
Conversation
….py to silence linter errors
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #719 +/- ##
=======================================
+ Coverage 76% 79% +3%
=======================================
Files 97 97
Lines 7874 7874
=======================================
+ Hits 5970 6223 +253
+ Misses 1904 1651 -253 🚀 New features to boost your workflow:
|
|
There was a problem hiding this comment.
Pull request overview
This PR adds explicit imports for ast in main.py and torch in datasets/_develop.py to resolve linter errors. While these imports are technically used in both files, adding the module-level torch import in _develop.py conflicts with the intentional lazy-loading design of the _SimpleDataset class.
Changes:
- Added
import asttosrc/rfdetr/main.py(used forast.literal_evalin argument parsing) - Added
import torchtosrc/rfdetr/datasets/_develop.py(needed fortorch.Tensortype hint)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/rfdetr/main.py | Adds missing ast import needed for ast.literal_eval call on line 1016 |
| src/rfdetr/datasets/_develop.py | Adds module-level torch import for type hint, but conflicts with lazy loading design |
| from typing import Any, Generator, Optional, Tuple | ||
| from urllib.request import urlretrieve | ||
|
|
||
| import torch |
There was a problem hiding this comment.
Adding a module-level torch import conflicts with the documented design intention of _SimpleDataset. The class docstring explicitly states that it "does not pull in torch at module load time" (lines 44-46), and torch was intentionally imported locally within __getitem__ (line 72) to achieve lazy loading.
While the module-level import is needed for linter compatibility with the torch.Tensor type hint on line 70, this breaks the lazy loading behavior. Consider one of these alternatives:
- Use a string literal for the type hint:
def __getitem__(self, idx: int) -> Tuple["torch.Tensor", dict]:combined withfrom __future__ import annotations(already present) - Import torch conditionally using TYPE_CHECKING:
if TYPE_CHECKING: import torch - Update the docstring to reflect that torch is now imported at module load time
Since this file already uses from __future__ import annotations (line 13), option 1 (string literal) would preserve the lazy loading behavior while satisfying linters.
I have accepted the license agreement several times, but it still shows up as not signed. This is my first PR for an open-source project, so I'm a bit new to this.
|
a39bebd to
a6e6ca0
Compare
could you please share the screenshot :) |
|
@anatoly-ryabchenko, friendly ping, do you think you will have a chance to finish this one? 🦝 |
Sorry for a very long delay. Had a very busy time, but would be happy to finish this and look at other issues in a day or so. Thanks for the ping! |
|
@anatoly-ryabchenko I can help, but since the CI is still waiting on the CLA, I would need you sharig prinscreen with you signed so I can move forward :) |
|
Thank you! |
…rrors (#719) * refactor: add explicit (non-dynamic) imports for _develop.py to silence linter errors * refactor: move `numpy`, `torch`, and `PIL` imports to top-level --------- Co-authored-by: Anatoly Ryabchenko <anatoly@linqapp.io> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: jirka <6035284+Borda@users.noreply.github.com>
…rrors (#719) * refactor: add explicit (non-dynamic) imports for _develop.py to silence linter errors * refactor: move `numpy`, `torch`, and `PIL` imports to top-level --------- Co-authored-by: Anatoly Ryabchenko <anatoly@linqapp.io> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: jirka <6035284+Borda@users.noreply.github.com>


What does this PR do?
import ast was missing from main.py
import torch was missing from datasets/_develop.py
this may not have caused runtime crashes, since dependencies were imported upstream, but it is a bad practice and adds linter errors. This PR fixes that.
Related Issue(s): N/A
Type of Change
Testing
Test details:
Linter errors disappear (tested with PyCharm), functionality not affected (training, export).
Checklist
Additional Context